-
-
Notifications
You must be signed in to change notification settings - Fork 236
feat: Support unique box moving #568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough新增 UniqueProvider 及 UniqueContext、useTargetState、useDelay、useOffsetStyle、FloatBg、MotionContent 等组件与钩子;重构 Trigger 与 Popup 的集成以支持“唯一弹层”流,Popup 增加 onResize/children,样式与示例、测试一并加入(样式为纯视觉/动画变更)。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 用户
participant T as Trigger
participant UP as UniqueProvider
participant P as Popup/Portal
participant MC as MotionContent
participant FB as FloatBg
Note over T,UP: 非 unique(原有)流
U->>T: Hover/Click
T->>T: useDelay 处理打开/关闭
T->>P: 渲染/更新 Popup
P-->>T: onAlign/onResize
alt unique=true(新流)
U->>T: Hover/Leave
T->>UP: show(options) / hide(delay)
UP->>UP: useTargetState 管理队列与动画态
UP->>P: 在 Portal 中渲染单一 Popup(可切换内容)
P->>MC: 包裹内容并 appear(motion)
P->>FB: 渲染 FloatBg(removeOnLeave=false)
P-->>UP: onResize -> UP 更新 popupSize 并触发对齐
UP-->>P: 应用 useOffsetStyle 计算的偏移样式
end
sequenceDiagram
autonumber
participant T1 as Trigger#1
participant T2 as Trigger#2
participant UP as UniqueProvider
participant P as 单一 Popup
T1->>UP: show({ id:A, popup:tooltip1 })
UP->>P: 打开并渲染 tooltip1(FloatBg 可见)
T2->>UP: show({ id:B, popup:tooltip2 })
UP->>P: 直接切换内容(无关闭动画)
P-->>UP: onVisibleChanged(仍为 true)
Note over UP,P: 保持单一 Popup 与 FloatBg
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant enhancement to the Trigger
component by enabling a 'unique box moving' feature. This allows multiple trigger elements to share and dynamically update a single, persistent floating popup, rather than each trigger managing its own. The core of this feature is the new UniqueProvider
component, which centralizes the popup's state, positioning, and animations. This change improves the user experience for interactive elements like tooltips or popovers that need to smoothly transition their content and position when the user interacts with different trigger points.
Highlights
- UniqueProvider Component: Introduced a new
UniqueProvider
component that allows multipleTrigger
instances to share a single, globally managed floating popup. This enables seamless transitions of popup content between different trigger elements without the popup disappearing and reappearing. - Unique Trigger Prop: The
Trigger
component now supports aunique
prop. When set totrue
and wrapped within aUniqueProvider
, the trigger will delegate its popup display to the provider, facilitating the 'moving popup' behavior. - Popup Styling and Motion Components: Added
FloatBg
andMotionContent
components within theUniqueProvider
to handle the background and content rendering of the unique popup, including fade animations and dynamic positioning. - Offset Styling Refactoring: Extracted the complex logic for calculating and applying popup offset styles into a new reusable
useOffsetStyle
hook, improving modularity and readability in thePopup
component. - Delay Management Hook: A new
useDelay
hook was added to centralize and manage delayed function calls (e.g.,setTimeout
), ensuring proper cleanup and preventing memory leaks. - New Demo and Tests: Included a new demo (
Moving Popup
) to visually showcase the unique box moving functionality and added dedicated test cases (unique.test.tsx
) to ensure its correct behavior.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a UniqueProvider
to enable a single, shared popup that can move between different trigger elements. This is a significant feature enhancement, implemented through a new context, several new components (UniqueProvider
, FloatBg
, MotionContent
), and custom hooks (useTargetState
, useDelay
, useOffsetStyle
). The code is well-structured, with logic cleanly extracted into hooks. The changes also include a helpful demo and a new test case. My review focuses on improving type safety, removing leftover debug code, and refining some implementation details for better maintainability.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #568 +/- ##
==========================================
- Coverage 97.75% 95.92% -1.84%
==========================================
Files 12 18 +6
Lines 801 932 +131
Branches 235 259 +24
==========================================
+ Hits 783 894 +111
- Misses 18 38 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (10)
assets/index.less (2)
82-84
: 移除明显的调试配色(红边、绿底),避免误入生产。这些样式看起来用于调试,会在 UI 上显著可见,建议用中性样式或变量占位,或仅在开发模式注入。
应用以下精简(同时增加 pointer-events 防止遮挡交互):
&-float-bg { position: absolute; z-index: 0; box-sizing: border-box; - border: 1px solid red; - background: green; + border: 0; + background: transparent; + pointer-events: none;
96-98
: 移除!important
,通过选择器权重或结构优化覆盖样式。
!important
会增加维护成本,应尽量避免。- &-unique-controlled { - border-color: rgba(0, 0, 0, 0.01) !important; - background: transparent !important; + &-unique-controlled { + border-color: rgba(0, 0, 0, 0.01); + background: transparent;src/hooks/useOffsetStyle.ts (1)
14-14
: 移除已完成的 TODO 注释。此文件正是将 offsetStyle 逻辑抽出到 Hook 的实现,TODO 已完成。
- // TODO: Move offsetStyle logic to useOffsetStyle.ts hooks
src/UniqueProvider/FloatBg.tsx (3)
12-12
: 为align
使用精确类型AlignType
,提升类型安全。当前为
any
,与 Hook 定义不一致。-import React from 'react'; +import React from 'react'; +import type { AlignType } from '../interface'; // ... export interface FloatBgProps { prefixCls: string; // ${prefixCls}-float-bg isMobile: boolean; ready: boolean; open: boolean; - align: any; + align: AlignType;
39-44
: 移除注释掉的临时代码,保持文件整洁。- // ========================= Ready ========================== - // const [delayReady, setDelayReady] = React.useState(false); - - // React.useEffect(() => { - // setDelayReady(ready); - // }, [ready]);
66-67
: 移除调试注释。多余注释会干扰阅读。
- // Remove console.log as it's for debugging only - // console.log('>>>', ready, open, offsetStyle); + //(已移除调试日志)src/context.ts (1)
2-2
: 为 UniqueShowOptions 去除 any,补全精确类型(重复建议)将
builtinPlacements/popupAlign/popupMotion/maskMotion/arrow
等从any
替换为具体类型,改善类型安全与智能提示。-import type { TriggerProps } from './index'; +import type { + TriggerProps, + BuildInPlacements, + AlignType, + ArrowTypeOuter, +} from './index'; +import type { CSSMotionProps } from '@rc-component/motion'; @@ - popupPlacement?: string; - builtinPlacements?: any; - popupAlign?: any; + popupPlacement?: string; + builtinPlacements?: BuildInPlacements; + popupAlign?: AlignType; zIndex?: number; mask?: boolean; maskClosable?: boolean; - popupMotion?: any; - maskMotion?: any; - arrow?: any; + popupMotion?: CSSMotionProps; + maskMotion?: CSSMotionProps; + arrow?: boolean | ArrowTypeOuter;Also applies to: 13-32
src/UniqueProvider/index.tsx (1)
111-123
: 未使用的 subPopupElements 存储(重复建议)仅注册从未读取,建议移除或补充用途,避免无谓状态。
src/UniqueProvider/useTargetState.ts (2)
37-39
: 注释表意不准(重复建议)这里的
setOptions(nextOptions)
并非“函数式更新”;建议简化注释,避免误导。- // Use functional update to ensure re-render is always triggered + // Set new options
54-56
: 同上,简化待应用选项的注释避免“functional update”措辞。
- // Apply pending options - Use functional update to ensure re-render is triggered + // Apply pending options
🧹 Nitpick comments (13)
assets/index.less (1)
89-91
: 避免使用transition: all
,限定过渡属性以减少重排与性能风险。若仅期望淡入/位移过渡,请明确具体属性。
- &-visible { - transition: all 0.1s; - } + &-visible { + transition: opacity 0.1s, transform 0.1s; + }src/hooks/useDelay.ts (1)
13-23
: 稳定回调引用并夹紧负延时,避免不必要重渲染与边界值问题。
- 用
useCallback
稳定delayInvoke
引用,降低依赖项波动;- 负数延时按 0 处理更健壮;
- 明确单位为“秒”,减少误用。
export default function useDelay() { const delayRef = React.useRef<ReturnType<typeof setTimeout> | null>(null); const clearDelay = () => { if (delayRef.current) { clearTimeout(delayRef.current); delayRef.current = null; } }; - const delayInvoke = (callback: VoidFunction, delay: number) => { + /** + * delay 单位:秒 + */ + const delayInvoke = React.useCallback((callback: VoidFunction, delay: number) => { clearDelay(); if (delay === 0) { callback(); } else { - delayRef.current = setTimeout(() => { + const ms = Math.max(0, delay * 1000); + delayRef.current = setTimeout(() => { callback(); - }, delay * 1000); + }, ms); } - }; + }, []);src/UniqueProvider/MotionContent.tsx (1)
11-32
: 可选:轻量场景下用React.memo
避免子树反复渲染。
visible
恒为 true 且仅appear
,组件重渲染主要来自父级。若children
内容稳定,可包一层memo
以减少无效渲染。-const MotionContent = (props: MotionContentProps) => { +const MotionContent = React.memo((props: MotionContentProps) => { // ... -}; +});src/hooks/useOffsetStyle.ts (1)
28-34
: 类型安全与健壮性小建议。
- 访问
align._experimental?.dynamicInset
使用as any
,可考虑在本仓库的AlignType
扩展一个可选字段,减少any
;points[0]
的字符访问依赖二字符格式(如br
、tl
),若存在非预期值建议在开发环境做断言。docs/examples/two-buttons.tsx (1)
49-58
: 去重 popupStyle,减少重复。两个 Trigger 使用相同样式,抽成常量更简洁。
-import Trigger, { UniqueProvider } from '@rc-component/trigger'; +import Trigger, { UniqueProvider } from '@rc-component/trigger'; import React, { useState } from 'react'; import '../../assets/index.less'; const LEAVE_DELAY = 0.2; +const popupStyle: React.CSSProperties = { + border: '1px solid #ccc', + padding: 10, + background: 'white', + boxSizing: 'border-box', +}; + // ... popup={<div>这是左侧按钮的提示信息</div>} - popupStyle={{ - border: '1px solid #ccc', - padding: 10, - background: 'white', - boxSizing: 'border-box', - }} + popupStyle={popupStyle} // ... popup={<div>This is the tooltip for the right button</div>} - popupStyle={{ - border: '1px solid #ccc', - padding: 10, - background: 'white', - boxSizing: 'border-box', - }} + popupStyle={popupStyle}Also applies to: 74-79
src/UniqueProvider/FloatBg.tsx (1)
88-96
: 可选:为容器添加无障碍与命中忽略属性。若 FloatBg 仅作视觉背景,建议
aria-hidden
或role="presentation"
,并配合 CSSpointer-events: none
(已在样式建议中),避免影响可达性与交互。- <div + <div className={cls} - style={{ + aria-hidden + role="presentation" + style={{ ...offsetStyle, ...sizeStyle, ...motionStyle, }} />tests/unique.test.tsx (3)
6-8
: 为global.openChangeLog
增加类型声明,避免 TS 下的隐式any
/属性不存在错误。-// Mock FloatBg to check if open props changed -global.openChangeLog = []; +// Mock FloatBg to check if open props changed +declare global { + // 记录 FloatBg open 属性变化 + // eslint-disable-next-line no-var + var openChangeLog: Array<{ from: boolean; to: boolean }>; +} +global.openChangeLog = [];
36-39
: 补充计时器与 mock 清理,降低测试间耦合。afterEach(() => { cleanup(); - jest.useRealTimers(); + jest.clearAllTimers(); + jest.useRealTimers(); + jest.clearAllMocks(); });
84-86
: 确认假定的延时推进步长足以覆盖mouseLeaveDelay
。当前
awaitFakeTimer()
每次推进 100ms 共 10 次,覆盖 0.1s 延时充足,但若未来修改mouseLeaveDelay
,建议直接推进目标时长,提升可读性与鲁棒性。src/Popup/index.tsx (3)
15-17
: useEvent 导入路径不一致,建议统一为 lib 子路径此处从包根导出拿
useEvent
,其它文件使用的是@rc-component/util/lib/hooks/useEvent
。为避免多入口导致的额外体积或多实例问题,建议统一到 lib 路径。-import useOffsetStyle from '../hooks/useOffsetStyle'; -import { useEvent } from '@rc-component/util'; +import useOffsetStyle from '../hooks/useOffsetStyle'; +import useEvent from '@rc-component/util/lib/hooks/useEvent';
65-66
: children 类型过窄,放宽为 ReactNode 提升可用性当前仅允许单个 ReactElement,不支持 Fragment、字符串或数组。建议改为
React.ReactNode
。- children?: React.ReactElement; + children?: React.ReactNode;
188-195
: Resize → onAlign 可能造成抖动/循环触发
onInternalResize
每次尺寸变化即调用onAlign
,若对齐样式影响布局,可能触发连续的 Resize。建议微任务/RAF 合并对齐调用。- const onInternalResize: ResizeObserverProps['onResize'] = useEvent( - (size, ele) => { - onResize?.(size, ele); - onAlign(); - }, - ); + const onInternalResize: ResizeObserverProps['onResize'] = useEvent( + (size, ele) => { + onResize?.(size, ele); + if (!resizeAlignLockRef.current) { + resizeAlignLockRef.current = true; + Promise.resolve().then(() => { + resizeAlignLockRef.current = false; + onAlign(); + }); + } + }, + );在上述变更处上方补充(同文件内):
// 放在 Resize 区块顶部 const resizeAlignLockRef = React.useRef(false);请在包含复杂动画或动态内容的场景下验证是否仍有循环触发。
src/context.ts (1)
22-23
: 可进一步收紧 popupPlacement 类型如有内置集合,考虑改为
keyof BuildInPlacements
提示合法值;若需支持自定义 placement,保持 string 不变也可。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
assets/index.less
(1 hunks)docs/demos/two-buttons.md
(1 hunks)docs/examples/two-buttons.tsx
(1 hunks)src/Popup/index.tsx
(10 hunks)src/UniqueProvider/FloatBg.tsx
(1 hunks)src/UniqueProvider/MotionContent.tsx
(1 hunks)src/UniqueProvider/index.tsx
(1 hunks)src/UniqueProvider/useTargetState.ts
(1 hunks)src/context.ts
(1 hunks)src/hooks/useDelay.ts
(1 hunks)src/hooks/useOffsetStyle.ts
(1 hunks)src/index.tsx
(10 hunks)tests/unique.test.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/UniqueProvider/MotionContent.tsx (1)
src/index.tsx (1)
TriggerProps
(49-130)
src/UniqueProvider/FloatBg.tsx (1)
src/hooks/useOffsetStyle.ts (1)
useOffsetStyle
(4-53)
src/UniqueProvider/index.tsx (4)
src/UniqueProvider/useTargetState.ts (1)
useTargetState
(14-66)src/hooks/useDelay.ts (1)
useDelay
(3-33)src/context.ts (4)
UniqueShowOptions
(14-32)UniqueContextProps
(34-37)TriggerContextProps
(5-7)UniqueContext
(39-41)src/hooks/useAlign.ts (1)
useAlign
(90-762)
tests/unique.test.tsx (1)
tests/util.tsx (1)
awaitFakeTimer
(97-104)
src/UniqueProvider/useTargetState.ts (1)
src/context.ts (1)
UniqueShowOptions
(14-32)
src/context.ts (1)
src/index.tsx (1)
TriggerProps
(49-130)
src/Popup/index.tsx (1)
src/hooks/useOffsetStyle.ts (1)
useOffsetStyle
(4-53)
docs/examples/two-buttons.tsx (1)
docs/examples/inside.tsx (1)
builtinPlacements
(12-96)
src/index.tsx (2)
src/context.ts (1)
UniqueContext
(39-41)src/hooks/useDelay.ts (1)
useDelay
(3-33)
🔇 Additional comments (11)
assets/index.less (1)
115-123
: 确认嵌套的@keyframes
构建产物不会被重复注入或作用域异常。Less 会将嵌套的
@keyframes
提升到顶层,但在多次引入样式或多个prefixCls
同时存在时,需确认无重复或命名冲突。src/hooks/useDelay.ts (1)
25-31
: 卸载清理完善,LGTM。在卸载时清理 pending 定时器,避免泄漏与“僵尸回调”。
src/UniqueProvider/MotionContent.tsx (1)
16-23
: motionName 与样式约定一致性请再确认一次。
motionName = ${prefixCls}-motion-content-fade
需要与 less 中的&-motion-content-&-fade-appear
规则一致(rc-motion 会加-appear
/-appear-active
后缀),当前看是匹配的,但请确保未遗漏导入样式。docs/demos/two-buttons.md (1)
1-8
: 文档条目 LGTM。Front matter 与示例引用路径看起来正确。
src/Popup/index.tsx (2)
246-246
: ResizeObserver 回调改为中转 onInternalResize 的方向是正确的同时支持外部
onResize
与内部onAlign
,并在!open
时禁用观察器,符合预期。
313-314
: Portal 中追加 {children} 的层级影响请确认
{children}
作为与弹层同级的后渲染节点,默认会叠在前面(同级后插入)。确保这些子元素(如 FloatBg)通过 z-index/position 明确在期望层级之下。src/index.tsx (1)
778-834
: Unique 分支渲染门控逻辑清晰存在 UniqueContext 且启用 unique 时不再本地渲染 Popup,避免双渲染。
src/UniqueProvider/index.tsx (3)
49-56
: 基于秒的延迟与动画安全的触发流程看起来合理
useDelay
统一处理 show/hide 的秒级延时,hide 不急于清空 options 以等待动画完成,符合预期。Also applies to: 58-63
160-166
: onResize 仅记录尺寸,避免业务耦合,LGTM后续计算(如 FloatBg)依赖
popupSize
,此处职责单一。
178-191
: FloatBg 层级与交互遮挡请确认作为 Popup 的
children
同级渲染,注意 z-index 与 pointer-events,避免覆盖弹层可交互区。src/UniqueProvider/useTargetState.ts (1)
26-47
: 状态机与动画过渡的设计清晰进入动画时缓存新 options,动画完成后再应用;隐藏时清空 pending。符合预期用例。
Also applies to: 49-66
useLayoutEffect(() => { | ||
if (uniqueContext && unique && targetEle && !openUncontrolled) { | ||
if (mergedOpen) { | ||
Promise.resolve().then(() => { | ||
uniqueContext.show(getUniqueOptions(0)); | ||
}); | ||
} else { | ||
uniqueContext.hide(0); | ||
} | ||
} | ||
}, [mergedOpen]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
受控模式下属性变更未同步到 UniqueProvider,可能导致内容/位置不更新
当前仅在 mergedOpen
改变时调用 uniqueContext.show/hide
。当受控打开后变更 popup
、placement、align、zIndex、mask、arrow 等属性时,UniqueProvider 不会收到更新。
在现有效果后追加一个 effect,当打开且受控时同步最新选项:
useLayoutEffect(() => {
if (uniqueContext && unique && targetEle && !openUncontrolled) {
if (mergedOpen) {
Promise.resolve().then(() => {
uniqueContext.show(getUniqueOptions(0));
});
} else {
uniqueContext.hide(0);
}
}
}, [mergedOpen]);
+// 受控+已打开时,属性变更也需要同步给 UniqueProvider
+useLayoutEffect(() => {
+ if (uniqueContext && unique && targetEle && !openUncontrolled && mergedOpen) {
+ uniqueContext.show(getUniqueOptions(0));
+ }
+ // 依赖列出会引发刷新:当这些选项变化时推送到 Provider
+}, [
+ uniqueContext,
+ unique,
+ targetEle,
+ mergedOpen,
+ openUncontrolled,
+ prefixCls,
+ popupClassName,
+ popupStyle,
+ popupPlacement,
+ builtinPlacements,
+ popupAlign,
+ zIndex,
+ mask,
+ maskClosable,
+ popupMotion,
+ maskMotion,
+ arrow,
+ getPopupContainer,
+]);
请补充相关用例覆盖:在受控打开态下修改 placement/align/popup 内容,期望 Unique 弹层即时更新。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useLayoutEffect(() => { | |
if (uniqueContext && unique && targetEle && !openUncontrolled) { | |
if (mergedOpen) { | |
Promise.resolve().then(() => { | |
uniqueContext.show(getUniqueOptions(0)); | |
}); | |
} else { | |
uniqueContext.hide(0); | |
} | |
} | |
}, [mergedOpen]); | |
useLayoutEffect(() => { | |
if (uniqueContext && unique && targetEle && !openUncontrolled) { | |
if (mergedOpen) { | |
Promise.resolve().then(() => { | |
uniqueContext.show(getUniqueOptions(0)); | |
}); | |
} else { | |
uniqueContext.hide(0); | |
} | |
} | |
}, [mergedOpen]); | |
// 受控+已打开时,属性变更也需要同步给 UniqueProvider | |
useLayoutEffect(() => { | |
if (uniqueContext && unique && targetEle && !openUncontrolled && mergedOpen) { | |
uniqueContext.show(getUniqueOptions(0)); | |
} | |
// 依赖列出会引发刷新:当这些选项变化时推送到 Provider | |
}, [ | |
uniqueContext, | |
unique, | |
targetEle, | |
mergedOpen, | |
openUncontrolled, | |
prefixCls, | |
popupClassName, | |
popupStyle, | |
popupPlacement, | |
builtinPlacements, | |
popupAlign, | |
zIndex, | |
mask, | |
maskClosable, | |
popupMotion, | |
maskMotion, | |
arrow, | |
getPopupContainer, | |
]); |
🤖 Prompt for AI Agents
In src/index.tsx around lines 331 to 342, the current effect only triggers
UniqueProvider updates when mergedOpen changes, so when the component is in
controlled-open state and props like popup, placement, align, zIndex, mask, or
arrow change the UniqueProvider won’t receive updated options; add a new
useEffect that runs when uniqueContext, unique, targetEle, openUncontrolled
(false), and any of the relevant props (popup, placement, align, zIndex, mask,
arrow, and any computed getUniqueOptions inputs) change and, when mergedOpen is
true (controlled open), call uniqueContext.show(getUniqueOptions(0)) (or
uniqueContext.hide if appropriate) to sync options; also add tests that mount
the component in controlled open state, mutate placement/align/popup content and
assert Unique's DOM/position/content updates immediately.
const prefixCls = options?.prefixCls; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefixCls 可能为 undefined,需提供默认值
Popup
的 prefixCls
为必需字符串;当前从 options?.prefixCls
透传可能为 undefined
,会影响类名与样式。
- const prefixCls = options?.prefixCls;
+ const prefixCls = options?.prefixCls ?? 'rc-trigger-popup';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const prefixCls = options?.prefixCls; | |
const prefixCls = options?.prefixCls ?? 'rc-trigger-popup'; |
🤖 Prompt for AI Agents
In src/UniqueProvider/index.tsx around lines 126 to 127, prefixCls is read as
const prefixCls = options?.prefixCls which can be undefined; make it default to
a safe string so Popup always receives a string (e.g. const prefixCls =
options?.prefixCls ?? 'unique-popup' or your project's standard prefix). Update
the assignment to coalesce undefined to the chosen default and ensure any
downstream typing reflects prefixCls as a non-optional string.
popup={ | ||
<MotionContent prefixCls={prefixCls} key={popupId}> | ||
{options.popup} | ||
</MotionContent> | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
当 options.popup 为函数时将导致 React 抛错(函数作为子节点)
这里把 options.popup
直接作为 <MotionContent>
的 children。若其为函数(合法类型),会作为 React 子节点渲染,抛出 “Functions are not valid as a React child”。
- popup={
- <MotionContent prefixCls={prefixCls} key={popupId}>
- {options.popup}
- </MotionContent>
- }
+ popup={
+ <MotionContent prefixCls={prefixCls} key={popupId}>
+ {typeof options.popup === 'function'
+ ? options.popup()
+ : options.popup}
+ </MotionContent>
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
popup={ | |
<MotionContent prefixCls={prefixCls} key={popupId}> | |
{options.popup} | |
</MotionContent> | |
} | |
popup={ | |
<MotionContent prefixCls={prefixCls} key={popupId}> | |
{typeof options.popup === 'function' | |
? options.popup() | |
: options.popup} | |
</MotionContent> | |
} |
🤖 Prompt for AI Agents
In src/UniqueProvider/index.tsx around lines 137 to 141, options.popup is passed
directly as children to <MotionContent>, which will throw if options.popup is a
function; guard against function children by checking typeof options.popup ===
'function' and calling it to obtain the React node (e.g. use options.popup(...)
or options.popup() as appropriate), otherwise render options.popup directly so
MotionContent always receives a valid React node.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/UniqueProvider/useTargetState.ts (2)
37-38
: 注释与实现不符:此处并非“函数式更新”
setOptions(nextOptions)
不是函数式更新,且也无需函数式更新来触发重渲染。建议简化注释以避免误导。(与先前机器人评论相同问题)- // Use functional update to ensure re-render is always triggered + // Set new options setOptions(nextOptions);
49-57
: 进入动画结束后应用待处理项 — LGTM逻辑清晰:先
setIsAnimating(false)
,再应用pendingOptionsRef
并清空。与先前建议“简化注释为 Apply pending options”一致。
🧹 Nitpick comments (3)
src/UniqueProvider/useTargetState.ts (3)
10-13
: 文档与实现语义需更精确对齐(仅等待进入动画,不处理离场队列)当前注释容易被理解为“切换 target 一律等待上一个弹层动画结束”。实现上仅在“进入动画时”缓存并延后应用;离场阶段并未做等待或排队。建议把注释改得更明确,避免误读。
可考虑如下微调注释:
- * a. We wait motion finish of previous popup. - * b. Then we set new target and show the popup. - * 4. During appear/enter animation, cache new options and apply after animation completes. + * a. 若当前处于出现/进入动画,等待动画完成后再切换; + * 若已稳定显示,则可直接切换到新 target(不等待离场动画)。 + * 4. 仅在 appear/enter 动画期间缓存新 options,动画完成后一次性应用。
17-19
: “cacheOptions” 命名与行为不完全匹配返回的第三项是“已应用”的最近一次 options(关闭时也保留),并非“待应用缓存”。建议更新注释或改名为 currentOptions/appliedOptions 以减少歧义;如果不改名,至少把注释改成“当前已应用的 options(可能为 null)”。
- /* Will always cache last which is not null */ - cacheOptions: UniqueShowOptions | null, + /* 当前已应用的 options;初始为 null,关闭时不清空以保留上次配置 */ + cacheOptions: UniqueShowOptions | null,
41-44
: 是否需要跟踪“离场动画”并在离场期间排队新请求?当前仅在进入动画(
!open -> open
)置isAnimating=true
;离场时未置位,意味着离场期间的再次trigger(newOptions)
会立即重开而不是等待离场完成。若产品期望“离场也等待”,需要在隐藏时标记离场动画,并在onVisibleChanged(false)
决策是否应用排队项;否则建议在注释中明确“仅等待进入动画”。Also applies to: 59-61
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/UniqueProvider/useTargetState.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/UniqueProvider/useTargetState.ts (1)
src/context.ts (1)
UniqueShowOptions
(14-32)
🔇 Additional comments (1)
src/UniqueProvider/useTargetState.ts (1)
26-31
: 确认关闭时保留 options 的预期当
nextOptions === false
时仅setOpen(false)
,未清空options
。这会让调用方在关闭后仍读到“上一次已应用的配置”。请确认这是对齐设计(例如用于复用上次配置)而非遗漏。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/index.tsx (1)
337-350
: 受控模式下仅监听 mergedOpen,属性变更未同步到 Provider(重复提醒)当受控且处于打开状态时,变更 popup/placement/align/zIndex/mask/arrow 等不会推送到 UniqueProvider,内容/位置可能不更新。需新增一个 effect 在受控+已开时同步最新选项。
useLayoutEffect(() => { if (uniqueContext && unique && targetEle && !openUncontrolled) { if (mergedOpen) { Promise.resolve().then(() => { uniqueContext.show(getUniqueOptions(0)); }); } else { uniqueContext.hide(0); } } }, [mergedOpen]); +// 受控 + 已打开:属性变化时也需同步给 UniqueProvider +useLayoutEffect(() => { + if (uniqueContext && unique && targetEle && !openUncontrolled && mergedOpen) { + uniqueContext.show(getUniqueOptions(0)); + } + // 这些依赖变化时推送最新选项 +}, [ + uniqueContext, + unique, + targetEle, + mergedOpen, + openUncontrolled, + prefixCls, + popupClassName, + popupStyle, + popupPlacement, + builtinPlacements, + popupAlign, + zIndex, + mask, + maskClosable, + popupMotion, + maskMotion, + arrow, + getPopupContainer, + popup, +]);另外请补充用例:受控打开态下修改 placement/align/popup,期望 Unique 弹层即时更新。
🧹 Nitpick comments (5)
src/index.tsx (5)
114-118
: unique 属性文档小措辞与默认值说明
- 注释建议:shared → share(“to share the floating popup”)。
- 补充“默认值:false”,便于使用方理解。
- /** - * Config with UniqueProvider to shared the floating popup. - */ + /** + * Work with UniqueProvider to share the floating popup. Default: false. + */ unique?: boolean;
289-296
: 避免每次渲染创建新的 arrow 对象(减少不必要更新)当前每次渲染都会创建新对象,可能导致依赖浅比较的子组件/Provider 误判变化。用 useMemo 稳定引用。
- const innerArrow: ArrowTypeOuter = arrow - ? { - // true and Object likely - ...(arrow !== true ? arrow : {}), - } - : null; + const innerArrow: ArrowTypeOuter = React.useMemo( + () => + arrow + ? { + // true and Object likely + ...(arrow !== true ? arrow : {}), + } + : null, + [arrow], + );
316-335
: UniqueProvider 选项组装基本完整;建议同步必要字段并稳定引用
- 如 UniqueProvider 需要“fresh/alignPoint/onPopupClick”等,请一并透传,避免行为偏差。
- 另可将
getUniqueOptions
返回对象通过useMemo
组合(或确保内部字段引用稳定),降低无效 diff。const getUniqueOptions = useEvent((delay: number = 0) => ({ popup, target: targetEle, delay, prefixCls, popupClassName, popupStyle, popupPlacement, builtinPlacements, popupAlign, zIndex, mask, maskClosable, popupMotion, maskMotion, arrow: innerArrow, getPopupContainer, + fresh, + // 若 Provider 侧支持以下字段也可补充: + // alignPoint, + // onPopupClick, id, }));
503-507
: forceAlign 在 Unique 模式下无效,应转由 Provider 侧对齐当前
forceAlign
仅调用本地triggerAlign
,在 Unique 模式下并未渲染本地 Popup,因此无效果。可在 Unique 路径下通过uniqueContext.show(getUniqueOptions(0))
触发一次刷新/对齐(若 Provider 提供更合适的realign/update
API 优先使用)。- React.useImperativeHandle(ref, () => ({ - nativeElement: externalForwardRef.current, - popupElement: externalPopupRef.current, - forceAlign: triggerAlign, - })); + React.useImperativeHandle(ref, () => ({ + nativeElement: externalForwardRef.current, + popupElement: externalPopupRef.current, + forceAlign: () => { + if (uniqueContext && unique) { + // 触发 Provider 侧重新计算(若有 realign API 可替换) + uniqueContext.show(getUniqueOptions(0)); + } else { + triggerAlign(); + } + }, + }));请确认 UniqueProvider 是否已有对齐专用 API(如
realign(id)
),若有可替换为该 API。
618-627
: Unique 模式下的“窗口点击关闭”判定可能与 Provider 重叠/冲突
useWinClick
的“是否点在弹层内”依赖popupEle/subPopupElements
。Unique 模式不渲染本地 Popup 时,这些引用为空,可能误判为外部点击而多触发一次关闭,与 Provider 的点击关闭逻辑叠加。
- 验证场景:Unique 模式 +
clickToHide
/maskClosable
,点击弹层内部是否被本地逻辑误判为外部点击。- 若复现:建议在 Unique 模式下让
useWinClick
处于“禁用”态(例如 hook 内根据disabled
参数直接 no-op),或让 UniqueContext 暴露“判断是否在 Unique 弹层内”的能力以复用。Also applies to: 779-834
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/UniqueProvider/FloatBg.tsx
(1 hunks)src/UniqueProvider/useTargetState.ts
(1 hunks)src/context.ts
(1 hunks)src/hooks/useOffsetStyle.ts
(1 hunks)src/index.tsx
(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/hooks/useOffsetStyle.ts
- src/context.ts
- src/UniqueProvider/useTargetState.ts
- src/UniqueProvider/FloatBg.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.tsx (2)
src/context.ts (1)
UniqueContext
(41-43)src/hooks/useDelay.ts (1)
useDelay
(3-33)
🔇 Additional comments (5)
src/index.tsx (5)
13-13
: 引入 UniqueContext 合理,解耦 Provider/Consumer该导入与后续使用保持一致,无需改动。
16-16
: useDelay 引入 OK与后文统一使用秒级 delay(乘以 1000)一致。
221-223
: 消费 UniqueContext 的位置合理就近读取上下文用于分支渲染与 show/hide 委托,结构清晰。
374-374
: 确认 delay 单位在 Trigger 与 UniqueProvider 之间一致此处
useDelay
以“秒”为单位(×1000)。getUniqueOptions(delay)
也传递秒。请确认 UniqueProvider 内部同样按“秒”解释 delay,避免节流/去抖时序误差。
35-36
: 确认 package.json 的 exports/typings 是否包含 UniqueProvidersrc/index.tsx 第35行已导出 UniqueProvider(export { default as UniqueProvider }),但未在 package.json 中发现 exports/types/typings 映射,可能导致使用方通过
import { UniqueProvider } from 'rc-trigger'
失败。请在仓库根目录确认并补充 exports/typings 映射或确保构建产物包含该导出。建议运行:# 检查是否从入口导出 UniqueProvider rg -nP "export\s+\{\s*default\s+as\s+UniqueProvider\s*\}" -g "src/**" # 全仓查找 package.json 中的 exports/types/typings/UniqueProvider 相关项 rg -n '"exports"|"types"|"typings"|UniqueProvider' -g '**/package.json'
// If it's controlled mode, always use internal trigger logic | ||
// UniqueProvider will be synced through useLayoutEffect | ||
if (popupVisible !== undefined) { | ||
delayInvoke(() => { | ||
internalTriggerOpen(nextOpen); | ||
}, delay * 1000); | ||
}, delay); | ||
return; | ||
} | ||
}; | ||
|
||
React.useEffect(() => clearDelay, []); | ||
// If UniqueContext exists and not controlled, pass delay to Provider instead of handling it internally | ||
if (uniqueContext && unique && openUncontrolled) { | ||
if (nextOpen) { | ||
uniqueContext.show(getUniqueOptions(delay)); | ||
} else { | ||
uniqueContext.hide(delay); | ||
} | ||
return; | ||
} | ||
|
||
delayInvoke(() => { | ||
internalTriggerOpen(nextOpen); | ||
}, delay); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unique 非受控路径未同步本地 open 状态,导致点击切换等逻辑失效
当 uniqueContext && unique && openUncontrolled
时,仅调用 Provider 的 show/hide
,未更新本地 internalOpen/openRef
。结果是打开后 openRef.current
仍为 false,点击触发不会走“点击隐藏”分支,表现为无法 toggle。
应用以下修复:在委托给 Provider 的同时,同步内部 open(与回调),可用同一 delay 保持时序一致。
- if (uniqueContext && unique && openUncontrolled) {
- if (nextOpen) {
- uniqueContext.show(getUniqueOptions(delay));
- } else {
- uniqueContext.hide(delay);
- }
- return;
- }
+ if (uniqueContext && unique && openUncontrolled) {
+ if (nextOpen) {
+ uniqueContext.show(getUniqueOptions(delay));
+ } else {
+ uniqueContext.hide(delay);
+ }
+ // 同步本地 open 状态与回调,保证交互判断正确(点击切换等)
+ delayInvoke(() => {
+ internalTriggerOpen(nextOpen);
+ }, delay);
+ return;
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// If it's controlled mode, always use internal trigger logic | |
// UniqueProvider will be synced through useLayoutEffect | |
if (popupVisible !== undefined) { | |
delayInvoke(() => { | |
internalTriggerOpen(nextOpen); | |
}, delay * 1000); | |
}, delay); | |
return; | |
} | |
}; | |
React.useEffect(() => clearDelay, []); | |
// If UniqueContext exists and not controlled, pass delay to Provider instead of handling it internally | |
if (uniqueContext && unique && openUncontrolled) { | |
if (nextOpen) { | |
uniqueContext.show(getUniqueOptions(delay)); | |
} else { | |
uniqueContext.hide(delay); | |
} | |
return; | |
} | |
delayInvoke(() => { | |
internalTriggerOpen(nextOpen); | |
}, delay); | |
}; | |
// If it's controlled mode, always use internal trigger logic | |
// UniqueProvider will be synced through useLayoutEffect | |
if (popupVisible !== undefined) { | |
delayInvoke(() => { | |
internalTriggerOpen(nextOpen); | |
}, delay); | |
return; | |
} | |
// If UniqueContext exists and not controlled, pass delay to Provider instead of handling it internally | |
if (uniqueContext && unique && openUncontrolled) { | |
if (nextOpen) { | |
uniqueContext.show(getUniqueOptions(delay)); | |
} else { | |
uniqueContext.hide(delay); | |
} | |
// 同步本地 open 状态与回调,保证交互判断正确(点击切换等) | |
delayInvoke(() => { | |
internalTriggerOpen(nextOpen); | |
}, delay); | |
return; | |
} | |
delayInvoke(() => { | |
internalTriggerOpen(nextOpen); | |
}, delay); | |
}; |
🤖 Prompt for AI Agents
In src/index.tsx around lines 377 to 399, when handling the branch for
uniqueContext && unique && openUncontrolled the code only calls
uniqueContext.show/hide and does not sync the component's local open state
(internalOpen / openRef) or trigger the open-change callbacks, which breaks
toggle behavior; fix by, when delegating to the Provider, also updating the
local open state and refs and calling the same internal open-change handler (use
the same delay mechanism) so the component's internalOpen/openRef and
onOpenChange are kept in sync with the Provider show/hide calls.
Summary by CodeRabbit